-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
report(lightwallet): render performance-budget section #8708
report(lightwallet): render performance-budget section #8708
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: current rendering is roughly this: https://lighthouse-dti65h1bz.now.sh/
This is just the basic table to start?
Seems like you'll have a followup that adds the "over budget" column and all that, yeah?
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
Updated. |
@paulirish is it possible to redeploy this with new master? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@khempenius this is looking good to me. Do you mind rebasing/merging from master so that this pulls in the change (#8870) to get performance-budget
results in the sample json? Should clean up some of the tests and make the auto deployed example include the table
…nto lightwallet_renderer3
woohoo! now we got budget section in the now deployment! https://lighthouse-gce0hkow5.now.sh/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good! Mostly style nits now
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
Outdated
Show resolved
Hide resolved
lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wohoo we are almost there!! 😮 🎉
lighthouse-core/report/html/renderer/performance-category-renderer.js
Outdated
Show resolved
Hide resolved
lighthouse-core/test/report/html/renderer/report-renderer-test.js
Outdated
Show resolved
Hide resolved
Updated. All resolved except this comment from @patrickhulce: FWIW I think I'd lean towards group since that's how we're doing everything else, but then again it might be a bit confusing that it's a single audit in the group so 🤷♂" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
🎉 🎉 |
Summary
Related Issues/PRs
#8675, #8427, #8709, #8708,#8522, #8539